feat: advisory cache default (CM-1328) - #4391
Conversation
PR SummaryMedium Risk Overview On a cache hit, the API still returns 202 but the job entry has Batch submit runs the cache check inside each job’s try/catch so a cache DB error marks only that job OpenAPI text is updated from “7-day cache not built” to describe this behavior. Bulk poll grouping uses Reviewed by Cursor Bugbot for commit 59c09c1. Bugbot is set up for automated code reviews on this repo. Configure here. |
ulemons
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds batch blast-radius submission/polling and reuses recent completed analyses to reduce duplicate Temporal workflows.
Changes:
- Adds batch submit and polling APIs with validation and partial-failure handling.
- Adds advisory-level cache lookup with force bypass.
- Updates routing, rate limits, tests, and OpenAPI documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
services/libs/data-access-layer/src/packages/blastRadius.ts |
Adds cache and batch queries. |
backend/src/api/public/v1/packages/submitBlastRadiusJobBatch.ts |
Implements batch submission. |
backend/src/api/public/v1/packages/submitBlastRadiusJobBatch.test.ts |
Tests batch submission behavior. |
backend/src/api/public/v1/packages/submitBlastRadiusJob.ts |
Adds cache reuse to single submissions. |
backend/src/api/public/v1/packages/submitBlastRadiusJob.test.ts |
Tests single-job caching. |
backend/src/api/public/v1/packages/getBlastRadiusJobBatch.ts |
Implements batch polling. |
backend/src/api/public/v1/packages/getBlastRadiusJobBatch.test.ts |
Tests batch polling. |
backend/src/api/public/v1/packages/blastRadiusBatch.ts |
Defines batch schemas and pagination. |
backend/src/api/public/v1/packages/blastRadiusBatch.test.ts |
Tests schemas and pagination. |
backend/src/api/public/v1/packages/blastRadius.ts |
Defines cache configuration and helper. |
backend/src/api/public/v1/akrites-external/openapi.yaml |
Documents caching and batch APIs. |
backend/src/api/public/v1/akrites-external/index.ts |
Registers routes and adjusts rate limiting. |
Comments suppressed due to low confidence (2)
backend/src/api/public/v1/akrites-external/openapi.yaml:460
- Cache-hit entries do not start workflows, so this cost explanation is inaccurate. Qualify the statement as applying to cache misses.
Capped much lower than the 100-item read batches — each entry
starts its own Temporal workflow, so the batch multiplies
workflow starts (and reachability-analysis cost) per request.
backend/src/api/public/v1/akrites-external/openapi.yaml:1137
- The batch shares the single-submit cache semantics, so only cache misses start workflows. The current absolute wording contradicts the implementation.
single package. Each entry starts its own Temporal workflow, so the
batch is capped at 20 jobs (10 recommended as the default batch
size), much lower than the 100-item read batches, and stays behind
the same strict rate limiter as the single-job route.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| candidates_considered, started_at, completed_at | ||
| FROM blast_radius_analyses | ||
| WHERE advisory_osv_id = $(advisoryOsvId) | ||
| AND package_name IS NOT DISTINCT FROM $(packageName) |
| WHERE advisory_osv_id = $(advisoryOsvId) | ||
| AND package_name IS NOT DISTINCT FROM $(packageName) | ||
| AND ecosystem = $(ecosystem) | ||
| AND status = 'done' | ||
| AND completed_at >= NOW() - make_interval(days => $(maxAgeDays)) |
| status and, once done, results. Bulk submit (jobs:batch) is capped at | ||
| 20 jobs per request (10 recommended as the default batch size) — each | ||
| entry starts its own workflow — and stays behind the same strict rate | ||
| limiter as the single-job route; bulk poll | ||
| (jobs:batch/poll) is read-only and capped at 100 like the other batch | ||
| endpoints. Same interim scope note as Advisories applies (read:packages, | ||
| pending a dedicated read:advisories scope). | ||
| limiter as the single-job route, defaulting to 50 requests/hour | ||
| (configurable via AKRITES_BLAST_RADIUS_RATE_LIMIT_MAX / _WINDOW_MS); |
4dfa3be to
ba7b467
Compare
Signed-off-by: Umberto Sgueglia <[email protected]>
Signed-off-by: Umberto Sgueglia <[email protected]>
Signed-off-by: Umberto Sgueglia <[email protected]>
Signed-off-by: Umberto Sgueglia <[email protected]>
bfd8a2b to
59c09c1
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 59c09c1. Configure here.
| nullable: true | ||
| allOf: | ||
| - $ref: '#/components/schemas/BlastRadiusAnalysis' | ||
|
|
There was a problem hiding this comment.
Duplicate OpenAPI batch schemas
Low Severity
This commit re-adds BlastRadiusJobBatchRequest, BlastRadiusJobBatchResponse, BlastRadiusJobPollBatchRequest, and BlastRadiusAnalysisBulkEntry, which already exist just above. YAML duplicate keys keep the later copies, and their BlastRadiusJobBatchResponse text differs from the earlier one, so the less accurate description wins and the two blocks can drift further apart.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 59c09c1. Configure here.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
services/libs/data-access-layer/src/packages/blastRadius.ts:197
- This lookup runs on every non-forced submit (up to 20 times per batch), but the table currently has indexes only on
advisory_id,package_id, andstatus(V1784700000__blast_radius_analyses.sql:37-42). PostgreSQL therefore has no index supportingadvisory_osv_id/package_name/ecosystemplus the recency ordering and must scan/sort matchingdonerows as analyses accumulate. Add a new migration with an appropriate composite partial index for this cache lookup.
WHERE advisory_osv_id = $(advisoryOsvId)
AND package_name IS NOT DISTINCT FROM $(packageName)
AND ecosystem = $(ecosystem)
AND status = 'done'
AND completed_at >= NOW() - make_interval(days => $(maxAgeDays))
backend/src/api/public/v1/akrites-external/openapi.yaml:456
- The new cache semantics documented here contradict the batch descriptions elsewhere in this OpenAPI file:
BlastRadiusJobBatchRequest.jobsand/jobs:batchstill say each entry starts its own workflow, although cache hits start none. Update those descriptions to distinguish fresh submissions from cached results so generated API documentation does not misstate cost and behavior.
Pending when a job is freshly submitted, returned before the Temporal
workflow runs. Done when the advisory cache is reused instead — see
force above — in which case analysisId is the cached analysis's own
id, already completed. In the batch submit response, a job whose
backend/src/api/public/v1/packages/blastRadius.ts:18
- The PR description advertises the configurable variable as
BLAST_RADIUS_CACHE_MAX_AGE_DAYS, but the implementation readsAKRITES_BLAST_RADIUS_CACHE_MAX_AGE_DAYS. Setting the documented name is silently ignored and falls back to one day. Align the implementation and documented/deployment variable name so operators can actually tune the cache window.
const blastRadiusCacheMaxAgeDaysEnv = Number(process.env.AKRITES_BLAST_RADIUS_CACHE_MAX_AGE_DAYS)
| allOf: | ||
| - $ref: '#/components/schemas/BlastRadiusAnalysis' | ||
|
|
||
| BlastRadiusJobBatchRequest: |
Summary
Adds bulk/batch endpoints for blast-radius analysis (
submitBlastRadiusJobBatch,getBlastRadiusJobBatch) alongside the existing single-job endpoints, and adds an advisory-level cache so a submit can reuse a recent'done'analysis for the same(advisoryId, package, ecosystem)instead of always starting a new Temporal workflow. Submitting many advisories at once (e.g. a batch of 20) no longer means 20 unconditional workflow starts — repeated/overlapping submissions within a configurable cache window (BLAST_RADIUS_CACHE_MAX_AGE_DAYS, default 1 day) short-circuit to the cached result.Changes
submitBlastRadiusJobBatch/getBlastRadiusJobBatch: new bulk endpoints mirroring the single-job ones. Batch size is capped lower than the other read-only batch endpoints (packages/advisories/contacts) and stays behind the strictblastRadiusRateLimiter, since each job can trigger a real Temporal workflow start, not just a read.workflow.start(or cache-lookup) failure insidesubmitBlastRadiusJobBatchreturns that entry asstatus: 'failed'without failing the rest of the batch — the cache lookup was moved inside the per-jobtry/catch(it was originally outside, which would have rejected the wholePromise.alland 500'd the entire batch on a transient DB error during the cache check).getRecentDoneAnalysis(new DAL function inservices/libs/data-access-layer/src/packages/blastRadius.ts): finds the most recent'done'analysis for the same(advisoryOsvId, packageName, ecosystem)within the cache window. UsesIS NOT DISTINCT FROMforpackageNamesince it's nullable (advisory-wide analyses have no package) and plain=never matchesNULL.getCachedJobEntry(new shared helper inblastRadius.ts): extracted so the cache-check-and-build-entry logic isn't duplicated betweensubmitBlastRadiusJobandsubmitBlastRadiusJobBatch's per-job handler — both now call the same function instead of maintaining two copies of the same lookup.force: trueon a request bypasses the cache entirely, same as before for the single-job endpoint.blastRadius.ts's row types to@crowd/types(per ADR-0006) or adding DAL integration tests/factories (per ADR-0007) in this PR — scoped decision to keep this change focused on the batch API + cache behavior.Type of change
JIRA ticket
CM-1328